Skip to content

http,net,stream: optimize outgoing write paths - #64987

Open
GetThatCookie wants to merge 1 commit into
nodejs:mainfrom
GetThatCookie:pr64980_write_path
Open

http,net,stream: optimize outgoing write paths#64987
GetThatCookie wants to merge 1 commit into
nodejs:mainfrom
GetThatCookie:pr64980_write_path

Conversation

@GetThatCookie

Copy link
Copy Markdown

This PR is one of four focused changes split out of #64980 following review
feedback.

The full context, rationale, related work, benchmarks, and validation details
are documented there.

The AI-assistance disclosure in #64980 applies to this split PR as well.

Signed-off-by: GetThatCookie <NimmenKeks@gmx.de>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net
  • @nodejs/performance
  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 3, 2026
@pimterry

pimterry commented Aug 4, 2026

Copy link
Copy Markdown
Member

I haven't reviewed this in full yet (thanks for breaking up #64980, but this PR is still massive) but I've done a quick skim.

For the chunk writing improvements (independent of corking) I don't think most of this is necessary. There is a big improvement here in combining HTTP chunks within the same tick, which is great, but most of the rest of the code seems to be managing merging low-level calls which we already do internally anyway.

Testing on node without these changes, repeated writes here are already just one writev call:

const http = require('http');
const net = require('net');

const origWritev = net.Socket.prototype._writev;
net.Socket.prototype._writev = function (chunks, cb) {
  if (this.trackWrites) {
    console.log(`_writev called with ${chunks.length} chunks on socket ${this.remoteAddress}:${this.remotePort}`);
  }
  return origWritev.call(this, chunks, cb);
};

const origWrite = net.Socket.prototype._write;
net.Socket.prototype._write = function (data, encoding, cb) {
  if (this.trackWrites) {
    console.log(`_write called with ${data.length} bytes on socket ${this.remoteAddress}:${this.remotePort}`);
  }
  return origWrite.call(this, data, encoding, cb);
};

const server = http.createServer((req, res) => {
  req.socket.trackWrites = true;
  for (let i = 0; i < 16; i++) res.write('x'.repeat(64));
  res.end();
});

server.listen(0, () => {
  http.get({ port: server.address().port }, (res) => {
    let bytes = 0;
    res.on('data', (d) => bytes += d.length);
    res.on('end', () => server.close());
  });
});

Shows a single writev of 16 chunks for the 16 separate res.write calls. We should optimize the chunks, but we don't need to optimize the writev calls - it's already batched anyway.

Can you try simplifying this to combine chunked writes (in terms of transfer-encoding chunks I mean) but without all the writev internal stream changes? I think you'll find you get roughly identical perf boost with 10% of the code changes.

That's separate from the cork fix, I'll look at that closer but it would be helpful to clean this up first so the remaining logic is easier to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants